Adds support for reading OTIOZ #85

jminor · 2024-11-10T06:44:00Z

This PR adds support for reading the embedded content.otio within an OTIOZ file.

The file type is inferred from the filename extension.

% raven filename.otioz

To accomplish this, Raven now depends on the minizip-ng submodule. If I'm understanding the minizip-ng API correctly, Raven seeks into the OTIOZ and decompresses only the content.otio into memory without attempting to do anything with the media in the OTIOZ.

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

jminor · 2024-11-10T06:51:06Z

Hmmm... it looks like I'll need some assistance getting CMake to build/find universal builds of some dependencies. Or do we need those at all? I'm guessing libssl and libcrypto are for decrypting password protected ZIP files (which OTIOZ doesn't support), so we might be able to just disable those two. I'm not sure about liblzma or libzstd.

[ 98%] Linking CXX executable raven
ld: warning: ignoring file '/opt/homebrew/Cellar/xz/5.6.3/lib/liblzma.5.dylib': found architecture 'arm64', required architecture 'x86_64'
ld: warning: ignoring file '/opt/homebrew/Cellar/zstd/1.5.6/lib/libzstd.1.5.6.dylib': found architecture 'arm64', required architecture 'x86_64'
ld: warning: ignoring file '/opt/homebrew/Cellar/openssl@3/3.3.2/lib/libssl.3.dylib': found architecture 'arm64', required architecture 'x86_64'
ld: warning: ignoring file '/opt/homebrew/Cellar/openssl@3/3.3.2/lib/libcrypto.3.dylib': found architecture 'arm64', required architecture 'x86_64'
Undefined symbols for architecture x86_64:
  "_ERR_get_error", referenced from:
      _mz_crypt_sha_begin in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_sha_update in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_sha_end in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_aes_encrypt_final in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_aes_decrypt_final in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_aes_set_key in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_hmac_init in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      ...
  "_EVP_CIPHER_CTX_ctrl", referenced from:
      _mz_crypt_aes_encrypt_final in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)
      _mz_crypt_aes_decrypt_final in libminizip.a[x86_64][14](mz_crypt_openssl.c.o)

jminor · 2024-11-10T06:54:58Z

and Windows is unhappy about this:

D:\a\raven\raven\libs\minizip-ng\third_party\zstd\build\VS2010\libzstd-dll\libzstd-dll.rc(4): fatal error RC1015: cannot open include file 'zstd.h'. [D:\a\raven\raven\build_win64\_deps\zstd-build\lib\libzstd_shared.vcxproj]

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

ThomasWilshaw · 2024-11-10T11:33:18Z

I'm also getting issues buiding on Windows with various unresolved external symbol errors e.g.:

app.obj : error LNK2019: unresolved external symbol mz_zip_reader_open_file referenced in function "class opentimelineio::v1_0::Timeline * __cdecl LoadOTIOZFile(class std::basic_string<char,struct std::char_traits<char>,class std::alloc ator<char> >)" (?LoadOTIOZFile@@YAPEAVTimeline@v1_0@opentimelineio@@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z) [D:\src\raven\build\raven.vcxproj]

darbyjohnston · 2024-11-10T19:54:54Z

I would disable all of the options, leaving just ZIP compression + file parsing. Which is hopefully enough for supporting .otioz? If so we should probably mention that in the docs:

https://opentimelineio.readthedocs.io/en/latest/tutorials/otio-filebundles.html

-DMZ_BZIP2=OFF
-DMZ_LZMA=OFF
-DMZ_ZSTD=OFF
-DMZ_LIBCOMP=OFF
-DMZ_FETCH_LIBS=OFF
-DMZ_PKCRYPT=OFF
-DMZ_WZAES=OFF
-DMZ_OPENSSL=OFF
-DMZ_BCRYPT=OFF
-DMZ_LIBBSD=OFF
-DMZ_ICONV=OFF

…cies Signed-off-by: Joshua Minor <joshm@pixar.com>

jminor · 2024-11-11T04:34:55Z

Okay great. By turning off all those options, now the macOS build works, but Windows is still having trouble linking...

ThomasWilshaw · 2024-11-11T15:42:57Z

A small bit of progress, if I add the line set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) everything compiles fine and things sort of work (on Windows this is). The only example otioz file I have doesn't open and gives this message:

LOG: Invalid OTIOZ: "C:\Users\Tom\Downloads\aws-picchu-edit-4096x2048.otioz": Unable to open entry: -109

but that does imply the library is at least working. I don't know why that worked or if that is a sensible thing to do but if nothing else it might indicate the problem? I'm not at all good with CMake.

I also noticed the drag and drop code only excepts .otio files so that will need updating (app.cpp:is_valid_file()).

ThomasWilshaw · 2024-11-13T22:13:39Z

With a bit of fiddling I've managed to get this all working on Windows. In CMakeLists.txt I had to set:

set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
set(MZ_ZLIB ON)

and remove your changes from the last commit (I'm not sure if that step is essential or not).

I also found a bug in the effects drawing code, InvisibleButtons must have x and y sizes greater than 0 and with the larger OTIO file being rendered this wasn't the case. I added a catch for that and everything seems to be working correctly:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f4a7feb..e215b82 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -66,17 +66,8 @@ if(NOT EMSCRIPTEN AND NOT WIN32)
   add_subdirectory("libs/glfw")
 endif()
 
-set(MZ_BZIP2 OFF)
-set(MZ_LZMA OFF)
-set(MZ_ZSTD OFF)
-set(MZ_LIBCOMP OFF)
-set(MZ_FETCH_LIBS OFF)
-set(MZ_PKCRYPT OFF)
-set(MZ_WZAES OFF)
-set(MZ_OPENSSL OFF)
-set(MZ_BCRYPT OFF)
-set(MZ_LIBBSD OFF)
-set(MZ_ICONV OFF)
+set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
+set(MZ_ZLIB ON)
 add_subdirectory("libs/minizip-ng")
 
 if(NOT EMSCRIPTEN)
diff --git a/timeline.cpp b/timeline.cpp
index ccddf0e..5d07e90 100644
--- a/timeline.cpp
+++ b/timeline.cpp
@@ -393,6 +393,11 @@ void DrawEffects(
     auto old_pos = ImGui::GetCursorPos();
     ImGui::SetCursorPos(render_pos);
 
+    // ImGui::InvisibleButton size x and y must be greater than 0.0.
+    // See definition of ImGui::InvisibleButton in imgui_widgets.cpp:
+    if (size.x <= 0.0 || size.y <= 0.0) {
+        return;
+    }
     ImGui::PushID(item);
     ImGui::BeginGroup();

ThomasWilshaw · 2024-11-13T22:17:43Z

Also it's possible these two issues are relevant for the Windows build issues:
zlib-ng/minizip-ng#475
msys2/MINGW-packages#14029

Apologies if I'm posting too much

jminor · 2024-11-13T22:32:26Z

Oh, this is great progress - all of this is 100% welcome and encouraged :)

Is the linking issue a static vs dynamic link issue? I've attempted to keep everything statically linked into Raven for ease of installation. On macOS I checked the result with otool -L raven to be sure it only references system libraries. Do you know how to check that on Windows?

Could you make a separate PR for the InvisibleButton fix?

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

jminor · 2024-11-14T06:42:41Z

Thanks @ThomasWilshaw - I think that works. Can you verify that this actually runs and reads an OTIOZ on Windows?

I also addressed your note about checking for otioz via drag and drop.

ThomasWilshaw · 2024-11-14T14:35:14Z

Haha almost, I had to set set(MZ_FETCH_LIBS ON) and set(BUILD_SHARED_LIBS OFF) and it all seems to work bar the InvisibleButton crash (I'll open a PR for that shortly). I ran the dependancy checker on Windows and got the following:

D:\src\raven\build\Debug>dumpbin /dependents raven.exe
Microsoft (R) COFF/PE Dumper Version 14.29.30148.0
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file raven.exe

File Type: EXECUTABLE IMAGE

  Image has the following dependencies:

    d3d11.dll
    D3DCOMPILER_47.dll
    dwmapi.dll
    KERNEL32.dll
    USER32.dll
    GDI32.dll
    SHELL32.dll
    ole32.dll
    MSVCP140D.dll
    IMM32.dll
    VCRUNTIME140D.dll
    VCRUNTIME140_1D.dll
    ucrtbased.dll

  Summary

        1000 .00cfg
      23E000 .data
        6000 .idata
       3F000 .pdata
      279000 .rdata
        D000 .reloc
        1000 .rsrc
      528000 .text
        1000 .tls

Which looks correct I think. Do OTIOZ files only ever use deflate compression or might we need some of the other compression formats?

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

jminor · 2024-11-14T15:45:59Z

Thanks for checking that. I added those two more settings just now.

Would you mind also checking if the pre-compiled build result for Windows runs? https://github.com/OpenTimelineIO/raven/actions/runs/11840470183

@ThomasWilshaw thanks so much for helping with this!

ThomasWilshaw · 2024-11-14T16:09:18Z

CMakeLists.txt

+set(MZ_LZMA OFF)
+set(MZ_ZSTD OFF)
+set(MZ_LIBCOMP OFF)
+set(MZ_FETCH_LIBS OFF)


I think this line needs to be removed as you set it to ON earlier

Removing this line fixes the build locally for me and you can see in the GH Acions build log ZLIB is disabled so I suspect it's the same issue there.

Signed-off-by: Joshua Minor <joshm@pixar.com>

ThomasWilshaw · 2024-11-14T18:25:11Z

Works like a charm :)

jminor added 4 commits November 9, 2024 22:37

Added minizip-ng submodule

df3d323

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

Stubs for loading OTIOZ

6edf5e5

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

Build and link minizip-ng

d375907

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

Reads content.otio from OTIOZ files

0c0b918

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

jminor mentioned this pull request Nov 10, 2024

Crashes if given an OTIOZ #79

Closed

Free memory allocated during OTIOZ decompression.

96cc8dd

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

Turned off majority of minizip-ng options to reduce unwanted dependen…

b2ef4a9

…cies Signed-off-by: Joshua Minor <joshm@pixar.com>

Attempting to fix Windows build

04ea393

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

jminor force-pushed the otioz branch from d03bb60 to 04ea393 Compare November 14, 2024 06:07

Validate input file path

63eb43d

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

ThomasWilshaw mentioned this pull request Nov 14, 2024

timeline: Only draw InvisibleButton if size greater than zero #87

Merged

Tweaking Windows build settings

cb245b8

Signed-off-by: Joshua Minor <jminor@users.noreply.github.com>

ThomasWilshaw reviewed Nov 14, 2024

View reviewed changes

Removed incorrect toggle

86be63e

Signed-off-by: Joshua Minor <joshm@pixar.com>

jminor merged commit 2039019 into OpenTimelineIO:main Nov 15, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for reading OTIOZ #85

Adds support for reading OTIOZ #85

jminor commented Nov 10, 2024

jminor commented Nov 10, 2024

jminor commented Nov 10, 2024

ThomasWilshaw commented Nov 10, 2024

darbyjohnston commented Nov 10, 2024

jminor commented Nov 11, 2024

ThomasWilshaw commented Nov 11, 2024 •

edited

Loading

ThomasWilshaw commented Nov 13, 2024

ThomasWilshaw commented Nov 13, 2024

jminor commented Nov 13, 2024

jminor commented Nov 14, 2024

ThomasWilshaw commented Nov 14, 2024

jminor commented Nov 14, 2024

ThomasWilshaw Nov 14, 2024

ThomasWilshaw Nov 14, 2024

ThomasWilshaw commented Nov 14, 2024

Adds support for reading OTIOZ #85

Adds support for reading OTIOZ #85

Conversation

jminor commented Nov 10, 2024

jminor commented Nov 10, 2024

jminor commented Nov 10, 2024

ThomasWilshaw commented Nov 10, 2024

darbyjohnston commented Nov 10, 2024

jminor commented Nov 11, 2024

ThomasWilshaw commented Nov 11, 2024 • edited Loading

ThomasWilshaw commented Nov 13, 2024

ThomasWilshaw commented Nov 13, 2024

jminor commented Nov 13, 2024

jminor commented Nov 14, 2024

ThomasWilshaw commented Nov 14, 2024

jminor commented Nov 14, 2024

ThomasWilshaw Nov 14, 2024

Choose a reason for hiding this comment

ThomasWilshaw Nov 14, 2024

Choose a reason for hiding this comment

ThomasWilshaw commented Nov 14, 2024

ThomasWilshaw commented Nov 11, 2024 •

edited

Loading